fix(cli): hide console windows on Windows for background/detached spawns - #3996

First pass at 22156fb5. This is a comment, not an approval. Two independent passes went into it, mine and Codex's (static, in a separate checkout), and I checked every point below at this head. CI is green (94 passed).

What holds up

• The four new flags are correct for their stdio. All four spawns use pipes or ignore, so libuv adds CREATE_NO_WINDOW and the child gets a hidden console of its own.
• The two dropped sites are right to drop.
  • previewLifecycle.ts passes a log fd, so libuv would skip CREATE_NO_WINDOW anyway, and DETACHED_PROCESS already leaves the child with no console.
  • openBrowser launches a GUI app.
• Everything else I traced from preview is either hidden already or never runs in the console-less background server. I walked the import graph from commands/preview.ts (449 modules) and checked every spawn in it:
  • studio-server's waveform, peakMap (×2), proxyTranscoder (×2), mediaMetadata and freezeFrame already set windowsHide;
  • engine, producer, parsers and lint have no unhidden spawns;
  • the Windows branches in orphanCleanup and processTree (PowerShell, taskkill) are hidden;
  • portUtils netstat is only reached from --list/--stop in the foreground (Codex reached the same conclusion);
  • the telemetry pmset/vm_stat calls are darwin-only;
  • apt-get in browser/manager is Linux-only;
  • Studio's registry install passes skipClipboard: true, so clip.exe never runs.
• The tests catch removal of each change. I removed each windowsHide in turn (dev mode, local studio, transcodeToMp4, measureAudio), and each removal fails its test. Dropping SIGHUP from the signal list fails 3 tests.
• Test runs: the touched CLI files pass 41 of 41, and studio-server's loudness tests pass 25 of 25.

Should fix

1. Studio's background removal still opens a visible console the first time it's used (utils/optionalPackages.ts:142). The description lists optionalPackages.ts npm as foreground-only, but this path runs inside the background server:

   • Studio's media route calls startBackgroundRemoval (studio-server/src/routes/media.ts:238);
   • that runs background-removal/pipeline (server/studioServer.ts:668);
   • which calls createSession → loadOptionalPackage("onnxruntime-node", "background removal") (inference.ts:71);
   • on first use that runs install → runNpm.

   On Windows runNpm spawns cmd.exe /d /s /c npm.cmd install onnxruntime-node@… --prefix … (npxCommand.ts:14) with piped stdio and no windowsHide. In the background server, which has no console, that cmd.exe gets a new visible console window for the whole install.

   Background is the default whenever the shell isn't interactive (preview.ts:632), so agent-driven previews take this path. Embedded mode is the normal installed-CLI path, and there the route runs in that console-less process.

   windowsHide: true on that spawn is safe for the foreground callers too (snapshot, transcribe and the embedder), because their output is piped either way. Codex found this too and rated it blocking. I'd call it should-fix because it only happens once per machine and the feature still works.

2. Nothing tests that the shutdown handler actually kills the dev server, and on Windows this PR makes it the only thing that does (preview.ts:1340). I replaced killProcessTree(child.pid) in shutdown with a no-op, and all 40 preview.test.ts tests still pass.

   • The new "reaps the dev server when the terminal closes (SIGHUP)" test uses an already-exited child with no pid. It checks that the listener is registered and removed, but never calls it.
   • Before this PR, a foreground dev server shared the user's console. Ctrl+C and closing the terminal reached it directly.
   • With CREATE_NO_WINDOW it sits in its own hidden console, as the description says. So on Windows, Ctrl+C and terminal close now stop it only through this listener's taskkill /T /F.

   A test with a pending child that has a pid would cover it. It would call the captured SIGHUP (or SIGINT) listener, assert that the tree kill ran (mock ../utils/orphanCleanup.js), and then emit exit. Codex found the SIGHUP half of this.

Where I disagreed with Codex

• Codex also flagged studio/vite.producer.ts:26 (bun run --filter @hyperframes/producer build with stdio: "pipe" and no windowsHide) as a background dev-mode console flash. I don't think it can flash after this PR. That build runs inside the Vite host, which now has a hidden console of its own. A child without windowsHide attaches to its parent's console, and Windows only creates a new console when the parent has none. Adding the flag there for consistency is harmless, but nothing currently opens a window from it.

Nit

• runDevMode, runLocalStudioMode and transcodeToMp4 are exported only so the tests can import them. That's fine, but a one-line comment on each would stop someone from treating them as public API.

— Somu

Reviewed at 22156fb5, the full diff plus the surrounding code in preview.ts and orphanCleanup.ts. This adds to the earlier review on this head, which I agree with. Approving.

What I checked:

• The four spawns (preview.ts runDevMode and runLocalStudioMode, init.ts transcodeToMp4, loudness.ts measureAudio) all use pipes or ignore. So windowsHide does what the description says: libuv adds CREATE_NO_WINDOW, and nothing redirects to an fd that would make it skip that.
• The SIGHUP change (preview.ts:1342, :1359): one STUDIO_CHILD_SHUTDOWN_SIGNALS list drives both once and off, so a signal can't be registered and then left behind. That leak is what the comment at :1357 warns about.
• killProcessTree (orphanCleanup.ts:28) is fire-and-forget, which suits a console-close handler. Windows gives the process a few seconds after CTRL_CLOSE_EVENT, and the tree kill only has to be started within that time, not finished.
• Dropping openBrowser and previewLifecycle is right, for the reasons in the description.

The earlier review's two points, which I'd take as follow-ups rather than blockers:

1. utils/optionalPackages.ts:142 still spawns npm with no windowsHide. The background server reaches it through background removal's first-use install, so the description's "foreground-only" is wrong for that one site. This isn't a regression from this PR, and it's a one-line fix.
2. Nothing calls the shutdown listener on a live child with a pid. On Windows that listener is now the only thing that stops the dev server on Ctrl+C or terminal close, so it deserves a test where the tree kill is mocked and asserted.

Verdict: APPROVE
Reasoning: A small, correct change, with a test for each flag that fails when the flag is removed. The remaining gaps are one pre-existing spawn and one missing test, not regressions.

— Rames Jusso